feat: lastore更新支持p2p协议的流量优化 by qiuzhiqian · Pull Request #332 · linuxdeepin/lastore-daemon

qiuzhiqian · 2026-03-17T05:53:38Z

当前http协议的更新走cdn平台，网络开销较大。更新模块新增p2p协议更新，可以从同局域网的机器中下载 lastore-daemon新增以下功能：
1.由P2PUpdateEnable控制控制中心传递优化功能的开关状态，由P2PUpdateSupport表示upgrade服务的可用性，并控制控制中心传递优化功能是否展示 2.由SetP2PUpdateEnable来给控制中心控制P2PUpdateEnable的状态
3.新增dconfig的UpgradeDeliveryEnabled配置记录用户的操作状态
4.每次lastore服务启动时将根据UpgradeDeliveryEnabled来尝试控制upgrade服务的状态。如果upgrade拉起成功，就将/var/lib/lastore/下面的相关源切换称upgradedelivery协议

github-actions · 2026-03-17T05:53:53Z

CLA Assistant Lite bot:
提交邮箱中包含我们的合作伙伴，但您似乎并非合作伙伴的成员或对接人，请联系相关对接人将您添加至组织之中，或由其重新发起 Pull Request。
The commit email domain belongs to one of our partners, but it seems you are not yet a member of the current organization, please contact the contact person to add you to the organization or let them submit the Pull Request.

xml seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You can retrigger this bot by commenting recheck in this Pull Request}

当前http协议的更新走cdn平台，网络开销较大。更新模块新增p2p协议更新，可以从同局域网的机器中下载 lastore-daemon新增以下功能： 1.由P2PUpdateEnable控制控制中心传递优化功能的开关状态，由P2PUpdateSupport表示upgrade服务的可用性，并控制控制中心传递优化功能是否展示 2.由SetP2PUpdateEnable来给控制中心控制P2PUpdateEnable的状态 3.新增dconfig的UpgradeDeliveryEnabled配置记录用户的操作状态 4.每次lastore服务启动时将根据UpgradeDeliveryEnabled来尝试控制upgrade服务的状态。如果upgrade拉起成功，就将/var/lib/lastore/下面的相关源切换称upgradedelivery协议 Task: https://pms.uniontech.com/story-view-40501.html

deepin-ci-robot · 2026-03-17T05:57:54Z

deepin pr auto review

代码审查报告

1. 语法逻辑问题

1.1 `UpdateP2pDefaultSourceDir` 函数中的错误处理逻辑问题

在 src/internal/system/update_type.go 中的 UpdateP2pDefaultSourceDir 函数存在严重的错误处理问题：

p2pSource, err := ioutil.TempFile("/tmp", "p2pSource.*.list")
defer os.Remove(p2pSource.Name())
//从SystemSource.d或SecuritySource.d中读取每个文件内容并将协议替换成delivery协议后存放到/tmp中
//这么做为了保证替换协议的原子性
files, err := ioutil.ReadDir(sourceDir)

问题：

p2pSource 创建后没有检查错误就直接使用
如果 ioutil.TempFile 失败，p2pSource 可能是 nil，后续调用 p2pSource.Name() 会导致 panic
在读取文件内容时，如果遇到错误会直接 return，但此时已经删除了原始目录(os.RemoveAll(sourceDir))，导致系统配置丢失

改进建议：

func UpdateP2pDefaultSourceDir(updateType UpdateType, upgradeDeliveryEnabled bool) error {
    if !upgradeDeliveryEnabled {
        return nil
    }
    var sourceDir string
    switch updateType {
    case SystemUpdate:
        sourceDir = SoftLinkSystemSourceDir
    case SecurityUpdate:
        sourceDir = SecuritySourceDir
    default:
        return fmt.Errorf("unsupported update type: %d", updateType)
    }
    
    // 创建临时文件
    p2pSource, err := ioutil.TempFile("/tmp", "p2pSource.*.list")
    if err != nil {
        logger.Warningf("Failed to create temp file: %v", err)
        return err
    }
    defer os.Remove(p2pSource.Name())
    
    // 读取源目录中的所有文件
    files, err := ioutil.ReadDir(sourceDir)
    if err != nil {
        logger.Warningf("Error reading dir: %s err:%v", sourceDir, err)
        return err
    }
    
    // 处理每个文件
    for _, file := range files {
        var content []byte
        filePath := filepath.Join(sourceDir, file.Name())
        
        // 读取文件内容
        if utils.IsSymlink(filePath) {
            targetPath, err := os.Readlink(filePath)
            if err != nil {
                logger.Warningf("Error read link: %s err:%v", filePath, err)
                continue
            }
            if !utils.IsFileExist(targetPath) {
                logger.Warningf("target file is not exist: %s", filePath)
                continue
            }
            content, err = ioutil.ReadFile(targetPath)
        } else {
            content, err = ioutil.ReadFile(filePath)
        }
        
        if err != nil {
            logger.Warningf("Error reading file: %v", err)
            // 继续处理其他文件，而不是直接返回
            continue
        }
        
        // 替换协议并写入临时文件
        newContent := strings.ReplaceAll(string(content), "https://", "delivery://")
        if _, err := p2pSource.Write([]byte(newContent)); err != nil {
            logger.Warningf("Error writing to temp file: %v", err)
            return err
        }
    }
    
    // 关闭临时文件以确保所有内容都已写入
    if err := p2pSource.Close(); err != nil {
        logger.Warningf("Error closing temp file: %v", err)
        return err
    }
    
    // 创建备份目录
    backupDir := sourceDir + ".bak"
    if err := os.Rename(sourceDir, backupDir); err != nil {
        logger.Warningf("Failed to backup source dir: %v", err)
        return err
    }
    
    // 创建新的源目录
    if err := os.MkdirAll(sourceDir, 0755); err != nil {
        logger.Warningf("Failed to create source dir: %v", err)
        // 尝试恢复备份
        _ = os.Rename(backupDir, sourceDir)
        return err
    }
    
    // 移动临时文件到新目录
    destPath := filepath.Join(sourceDir, filepath.Base(p2pSource.Name()))
    if err := utils.MoveFile(p2pSource.Name(), destPath); err != nil {
        logger.Warningf("Failed to move temp file: %v", err)
        // 尝试恢复备份
        _ = os.RemoveAll(sourceDir)
        _ = os.Rename(backupDir, sourceDir)
        return err
    }
    
    // 如果一切成功，删除备份
    _ = os.RemoveAll(backupDir)
    
    return nil
}

1.2 `refreshUpgradeDeliveryService` 函数中的逻辑问题

在 src/lastore-daemon/updater.go 中的 refreshUpgradeDeliveryService 函数存在逻辑问题：

// 情况一：当upgrade服务开启但P2PUpdateEnable关闭时，尝试关闭服务。若关闭失败则将P2PUpdateEnable恢复为true
if !u.P2PUpdateEnable && v == system.UpgradeDeliveryEnable {
    err = object.Call("org.deepin.upgradedelivery.DisableService", 0).Err
    if err != nil {
        logger.Warning(err)
        u.setPropP2PUpdateEnable(true)
    } else {
        err = object.Call("org.deepin.upgradedelivery.Clear", 0).Err
    }
    return
}

问题：

如果 DisableService 成功但 Clear 失败，函数会直接返回，没有处理 Clear 的错误
在情况三中，如果 StartService 失败，只是设置属性为 false，但没有尝试清理资源

改进建议：

func (u *Updater) refreshUpgradeDeliveryService() {
    // 检查upgrade服务是否被正常安装
    _, err := u.service.NameHasOwner("org.deepin.upgradedelivery")
    if err != nil {
        logger.Warning("Upgrade delivery service not available:", err)
        u.setPropP2PUpdateSupport(false)
        return
    }
    
    object := u.service.Conn().Object("org.deepin.upgradedelivery", "/org/deepin/upgradedelivery")
    var ret dbus.Variant
    
    ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
    defer cancel()
    
    err = object.CallWithContext(ctx, "org.deepin.upgradedelivery.ServiceStatus", 0).Store(&ret)
    if err != nil {
        logger.Warning("Failed to get upgrade delivery service status:", err)
        u.setPropP2PUpdateSupport(false)
        return
    }
    
    u.setPropP2PUpdateSupport(true)
    v := ret.Value()
    
    // 根据配置和服务状态进行同步
    switch {
    // 情况一：服务开启但配置关闭，尝试关闭服务
    case !u.P2PUpdateEnable && v == system.UpgradeDeliveryEnable:
        if err := object.Call("org.deepin.upgradedelivery.DisableService", 0).Err; err != nil {
            logger.Warning("Failed to disable upgrade delivery service:", err)
            u.setPropP2PUpdateEnable(true)
            return
        }
        if err := object.Call("org.deepin.upgradedelivery.Clear", 0).Err; err != nil {
            logger.Warning("Failed to clear upgrade delivery cache:", err)
        }
        
    // 情况二：服务开启但配置未设置，同步状态
    case v == system.UpgradeDeliveryEnable:
        u.setPropP2PUpdateEnable(true)
        
    // 情况三：服务关闭但配置开启，尝试启动服务
    case u.P2PUpdateEnable && v != system.UpgradeDeliveryEnable:
        if err := object.Call("org.deepin.upgradedelivery.StartService", 0).Err; err != nil {
            logger.Warning("Failed to start upgrade delivery service:", err)
            u.setPropP2PUpdateEnable(false)
        }
    }
}

2. 代码质量问题

2.1 使用已弃用的函数

在 src/internal/system/update_type.go 中使用了 ioutil 包中的函数，这些函数在 Go 1.16+ 中已被弃用：

p2pSource, err := ioutil.TempFile("/tmp", "p2pSource.*.list")
files, err := ioutil.ReadDir(sourceDir)
content, err = ioutil.ReadFile(filePath)

改进建议：

// 使用 os 和 io 包替代 ioutil
p2pSource, err := os.CreateTemp("/tmp", "p2pSource.*.list")
files, err := os.ReadDir(sourceDir)
content, err = os.ReadFile(filePath)

2.2 变量命名不一致

在 src/lastore-daemon/updater.go 中，字段名从 p2PUpdateEnable 改为 P2PUpdateEnable，这破坏了命名一致性：

// 旧代码
p2PUpdateEnable  bool // p2p更新是否开启
p2PUpdateSupport bool // 是否支持p2p更新

// 新代码
P2PUpdateEnable  bool // p2p更新是否开启
P2PUpdateSupport bool // 是否支持p2p更新

改进建议：
保持一致的命名风格，建议使用 P2PUpdateEnable 和 P2PUpdateSupport，因为它们是导出字段，应该首字母大写。

2.3 缺少错误处理

在 src/internal/config/config.go 中，getConfigFromDSettings 函数添加了新的配置项，但没有充分处理错误：

_, err = c.dsLastoreManager.ConnectValueChanged(func(key string) {
    switch key {
    case dSettingsKeyUpgradeDeliveryEnabled:
        updateUpgradeDeliveryEnabled()
    }
})
if err != nil {
    logger.Warning(err)
}

改进建议：

// 添加更详细的错误日志
_, err = c.dsLastoreManager.ConnectValueChanged(func(key string) {
    switch key {
    case dSettingsKeyUpgradeDeliveryEnabled:
        updateUpgradeDeliveryEnabled()
    }
})
if err != nil {
    logger.Warningf("Failed to connect value changed for %s: %v", dSettingsKeyUpgradeDeliveryEnabled, err)
    // 设置默认值
    c.UpgradeDeliveryEnabled = false
}

3. 代码性能问题

3.1 字符串替换效率低

在 src/internal/system/update_type.go 和 src/internal/updateplatform/message_report.go 中，使用 strings.ReplaceAll 进行字符串替换：

newContent = strings.ReplaceAll(string(content), "https://", "delivery://")
repo.Source = strings.ReplaceAll(repo.Source, "https://", "delivery://")
repo.Uri = strings.ReplaceAll(repo.Uri, "https://", "delivery://")

改进建议：

对于大文件，考虑使用 bytes.Replace 或流式处理
对于多个替换操作，可以考虑使用正则表达式一次性替换

// 使用正则表达式一次性替换
var httpsRegex = regexp.MustCompile(`https://`)

// 在 UpdateP2pDefaultSourceDir 中
newContent := httpsRegex.ReplaceAllString(string(content), "delivery://")

// 在 genDepositoryFromPlatform 中
if m.config.UpgradeDeliveryEnabled {
    repo.Source = httpsRegex.ReplaceAllString(repo.Source, "delivery://")
    repo.Uri = httpsRegex.ReplaceAllString(repo.Uri, "delivery://")
}

3.2 文件操作效率低

在 UpdateP2pDefaultSourceDir 函数中，对每个文件都进行单独的读写操作：

for _, file := range files {
    var content []byte
    filePath := filepath.Join(sourceDir, file.Name())
    if utils.IsSymlink(filePath) {
        // ...
        content, err = ioutil.ReadFile(targetPath)
    } else {
        content, err = ioutil.ReadFile(filePath)
    }
    // ...
    newContent = strings.ReplaceAll(string(content), "https://", "delivery://")
    _, err = p2pSource.Write([]byte(newContent))
    // ...
}

改进建议：

使用缓冲区减少 I/O 操作
考虑使用 goroutine 并行处理多个文件

// 使用缓冲写入
buf := bufio.NewWriter(p2pSource)
defer buf.Flush()

for _, file := range files {
    // ...
    newContent := httpsRegex.ReplaceAllString(string(content), "delivery://")
    if _, err := buf.WriteString(newContent); err != nil {
        logger.Warningf("Error writing to buffer: %v", err)
        continue
    }
}

4. 代码安全问题

4.1 文件权限问题

在 UpdateP2pDefaultSourceDir 函数中，创建目录时使用了固定的权限 0755：

err = os.MkdirAll(sourceDir, 0755)

改进建议：
使用更安全的权限设置，并确保文件所有权正确：

// 使用更严格的权限
err = os.MkdirAll(sourceDir, 0750)
if err != nil {
    logger.Warningf("Failed to create directory: %v", err)
    return err
}

// 确保文件所有权正确
if err := os.Chown(sourceDir, 0, 0); err != nil {
    logger.Warningf("Failed to set ownership: %v", err)
    return err
}

4.2 临时文件安全问题

在 UpdateP2pDefaultSourceDir 函数中，临时文件创建在 /tmp 目录：

p2pSource, err := ioutil.TempFile("/tmp", "p2pSource.*.list")

改进建议：

使用更安全的临时目录
设置适当的文件权限

// 使用系统安全的临时目录
tmpDir := filepath.Join(os.TempDir(), "lastore-upgrade")
if err := os.MkdirAll(tmpDir, 0700); err != nil {
    logger.Warningf("Failed to create temp dir: %v", err)
    return err
}

// 创建临时文件
p2pSource, err := os.CreateTemp(tmpDir, "p2pSource.*.list")
if err != nil {
    logger.Warningf("Failed to create temp file: %v", err)
    return err
}

// 设置严格的文件权限
if err := p2pSource.Chmod(0600); err != nil {
    logger.Warningf("Failed to set temp file permissions: %v", err)
    p2pSource.Close()
    os.Remove(p2pSource.Name())
    return err
}

4.3 D-Bus 权限控制

在 src/lastore-daemon/updater_ifc.go 中添加了新的 D-Bus 方法，但权限控制可能不够严格：

func (u *Updater) SetP2PUpdateEnable(sender dbus.Sender, enable bool) *dbus.Error {
    err := polkit.CheckAuth(polkitActionChangeUpgradeDelivery, string(sender), nil)
    if err != nil {
        logger.Warning(err)
        return dbusutil.ToError(err)
    }
    // ...
}

改进建议：

添加额外的验证逻辑
记录操作日志

func (u *Updater) SetP2PUpdateEnable(sender dbus.Sender, enable bool) *dbus.Error {
    // 验证发送者
    uid, err := u.service.GetConnUID(string(sender))
    if err != nil {
        logger.Warningf("Failed to get sender UID: %v", err)
        return dbusutil.ToError(err)
    }
    
    // 记录操作
    logger.Infof("User %d attempting to set P2PUpdateEnable to %v", uid, enable)
    
    // 检查权限
    err = polkit.CheckAuth(polkitActionChangeUpgradeDelivery, string(sender), nil)
    if err != nil {
        logger.Warningf("Permission denied for user %d: %v", uid, err)
        return dbusutil.ToError(err)
    }
    
    // 执行操作
    err = u.config.SetUpgradeDeliveryEnabled(enable)
    if err != nil {
        logger.Warningf("Failed to set UpgradeDeliveryEnabled: %v", err)
        return dbusutil.ToError(err)
    }
    
    u.setPropP2PUpdateEnable(enable)
    logger.Infof("Successfully set P2PUpdateEnable to %v for user %d", enable, uid)
    return nil
}

5. 其他建议

5.1 添加单元测试

建议为新增的功能添加单元测试，特别是 UpdateP2pDefaultSourceDir 和 refreshUpgradeDeliveryService 这两个关键函数。

5.2 添加文档注释

为新增的函数和结构体添加详细的文档注释，说明其用途、参数和返回值。

5.3 考虑使用上下文(Context)

在涉及外部调用的函数中，建议使用 context 来控制超时和取消操作，例如：

func (u *Updater) refreshUpgradeDeliveryServiceWithContext(ctx context.Context) error {
    // ...
    object := u.service.Conn().Object("org.deepin.upgradedelivery", "/org/deepin/upgradedelivery")
    var ret dbus.Variant
    
    err = object.CallWithContext(ctx, "org.deepin.upgradedelivery.ServiceStatus", 0).Store(&ret)
    if err != nil {
        return fmt.Errorf("failed to get service status: %w", err)
    }
    // ...
}

总结

这次代码审查发现了几个关键问题，主要集中在错误处理、代码安全性和性能方面。建议优先处理 UpdateP2pDefaultSourceDir 函数中的错误处理逻辑问题，因为这个问题可能导致系统配置丢失。同时，建议改进临时文件的处理方式，以提高代码的安全性和可靠性。

deepin-ci-robot · 2026-03-17T06:15:56Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: qiuzhiqian, zhaohuiw42

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

qiuzhiqian force-pushed the feat-p2p-optimize branch from c9a6b3b to 9fe01b3 Compare March 17, 2026 05:56

zhaohuiw42 approved these changes Mar 17, 2026

View reviewed changes

qiuzhiqian merged commit 6d173cf into master Mar 17, 2026
24 of 29 checks passed

qiuzhiqian deleted the feat-p2p-optimize branch March 17, 2026 06:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: lastore更新支持p2p协议的流量优化#332

feat: lastore更新支持p2p协议的流量优化#332
qiuzhiqian merged 1 commit intomasterfrom
feat-p2p-optimize

qiuzhiqian commented Mar 17, 2026

Uh oh!

github-actions bot commented Mar 17, 2026 •

edited

Loading

Uh oh!

deepin-ci-robot commented Mar 17, 2026

Uh oh!

deepin-ci-robot commented Mar 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

qiuzhiqian commented Mar 17, 2026

Uh oh!

github-actions bot commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

deepin-ci-robot commented Mar 17, 2026

deepin pr auto review

代码审查报告

1. 语法逻辑问题

1.1 UpdateP2pDefaultSourceDir 函数中的错误处理逻辑问题

1.2 refreshUpgradeDeliveryService 函数中的逻辑问题

2. 代码质量问题

2.1 使用已弃用的函数

2.2 变量命名不一致

2.3 缺少错误处理

3. 代码性能问题

3.1 字符串替换效率低

3.2 文件操作效率低

4. 代码安全问题

4.1 文件权限问题

4.2 临时文件安全问题

4.3 D-Bus 权限控制

5. 其他建议

5.1 添加单元测试

5.2 添加文档注释

5.3 考虑使用上下文(Context)

总结

Uh oh!

deepin-ci-robot commented Mar 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Mar 17, 2026 •

edited

Loading

1.1 `UpdateP2pDefaultSourceDir` 函数中的错误处理逻辑问题

1.2 `refreshUpgradeDeliveryService` 函数中的逻辑问题